[no-release-notes] Update various query plan tests#2853
Conversation
|
|
|
SummaryThis run exercised core SQL behavior around join planning, catalog lookups, index-based filtering, and consistency of query results across startup timing and analyzer changes, covering both normal and edge-form query shapes. Overall behavior is stable for correctness-focused paths, with one regression observed in optimization behavior rather than returned data. Merge with caution — a medium-severity regression introduced by this PR causes some cast-shaped queries to lose efficient index pushdown, which can hurt performance even when results remain correct. Since the issue is in changed planner behavior rather than unrelated areas, it is a meaningful risk but not an immediate correctness blocker. Tests run by ItoTip Reply with @itoqa to send us feedback on this test run. |
| github.com/dolthub/eventsapi_schema v0.0.0-20260310172945-37a9265ade69 | ||
| github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260615190047-8d437133a8e6 | ||
| github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7 |
There was a problem hiding this comment.
Cast-wrapped predicates lose index pushdown
What failed: Expected cast-aware normalization to preserve selective pushdown behavior for logically equivalent predicates, but cast-wrapped form degraded to Filter over broad IndexedTableAccess(test.pk) while the uncasted form used test.v2 index filtering.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: Medium
- Impact: Cast-wrapped joins can lose the intended pushdown optimization, forcing broader scans and slower query execution even though the result rows stay correct.
- Steps to Reproduce:
- Start Doltgres from the repository and connect with psql as the local postgres user.
- Create and seed test and jointable fixtures with indexes, including rows where t.v2 = 22.
- Run EXPLAIN for an uncasted join predicate (t.v1 = j.v3 AND t.v2 = 22) and note index pushdown on test.v2.
- Run EXPLAIN for the cast-wrapped equivalent and compare plan shape.
- Observe that row results stay the same but the cast-wrapped plan uses a broad test.pk index range plus residual filter instead of the selective pushdown path.
- Stub / mock content: Local SQL authentication was intentionally bypassed by disabling SCRAM challenge/verification so planner behavior could be exercised in the QA environment; no mock query results were injected.
- Code Analysis: The PR diff directly changes dependency behavior by upgrading go-mysql-server in go.mod:12. Doltgres wires a custom splitter in server/analyzer/init.go:129-133, but the implementation in server/analyzer/split.go:30-47 only recursively splits expression.And and unwraps pgexprs.GMSCast; other wrapper forms are treated as opaque. The costed scan normalization path is similarly narrow in server/analyzer/split.go:58-64 and is used by servercfg/config.go:71, so equivalent wrapped predicates can miss pushdown opportunities. A targeted fix is to extend SplitConjunction and LogicTreeWalker to unwrap additional analyzer wrapper forms introduced by the upgraded dependency (or temporarily pin/revert the dependency bump until that normalization is added).
- Why this is likely a bug: The failing behavior is reproducible on logically equivalent queries and is consistent with a concrete code limitation in expression normalization, not a harness-only setup issue. The observed row parity with degraded plan shape matches a real planner regression where performance-relevant pushdown semantics are lost for cast-wrapped forms.
Relevant code
go.mod:12
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7server/analyzer/split.go:30-47
switch expr := expr.(type) {
case *expression.And:
return append(SplitConjunction(ctx, expr.LeftChild), SplitConjunction(ctx, expr.RightChild)...)
case *pgexprs.GMSCast:
split := SplitConjunction(ctx, expr.Child())
...
default:
return []sql.Expression{expr}
}server/analyzer/split.go:58-64
func (l *LogicTreeWalker) Next(e sql.Expression) sql.Expression {
switch expr := e.(type) {
case *pgexprs.GMSCast:
return l.Next(expr.Child())
default:
return e
}
}server/analyzer/init.go:129-133
analyzer.SplitConjunction = SplitConjunction
memo.SplitConjunction = SplitConjunctionEvidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**Medium severity — Cast-wrapped predicates lose index pushdown**
**What failed:** Expected cast-aware normalization to preserve selective pushdown behavior for logically equivalent predicates, but cast-wrapped form degraded to Filter over broad IndexedTableAccess(test.pk) while the uncasted form used test.v2 index filtering.
- **Impact:** Cast-wrapped joins can lose the intended pushdown optimization, forcing broader scans and slower query execution even though the result rows stay correct.
- **Steps to reproduce:**
1. Start Doltgres from the repository and connect with psql as the local postgres user.
2. Create and seed test and jointable fixtures with indexes, including rows where t.v2 = 22.
3. Run EXPLAIN for an uncasted join predicate (t.v1 = j.v3 AND t.v2 = 22) and note index pushdown on test.v2.
4. Run EXPLAIN for the cast-wrapped equivalent and compare plan shape.
5. Observe that row results stay the same but the cast-wrapped plan uses a broad test.pk index range plus residual filter instead of the selective pushdown path.
- **Stub / mock content:** Local SQL authentication was intentionally bypassed by disabling SCRAM challenge/verification so planner behavior could be exercised in the QA environment; no mock query results were injected.
- **Code analysis:** The PR diff directly changes dependency behavior by upgrading go-mysql-server in go.mod:12. Doltgres wires a custom splitter in server/analyzer/init.go:129-133, but the implementation in server/analyzer/split.go:30-47 only recursively splits expression.And and unwraps pgexprs.GMSCast; other wrapper forms are treated as opaque. The costed scan normalization path is similarly narrow in server/analyzer/split.go:58-64 and is used by servercfg/config.go:71, so equivalent wrapped predicates can miss pushdown opportunities. A targeted fix is to extend SplitConjunction and LogicTreeWalker to unwrap additional analyzer wrapper forms introduced by the upgraded dependency (or temporarily pin/revert the dependency bump until that normalization is added).
- **Why this is likely a bug:** The failing behavior is reproducible on logically equivalent queries and is consistent with a concrete code limitation in expression normalization, not a harness-only setup issue. The observed row parity with degraded plan shape matches a real planner regression where performance-relevant pushdown semantics are lost for cast-wrapped forms.
**Relevant code:**
`go.mod:12`
~~~go
github.com/dolthub/go-mysql-server v0.20.1-0.20260618182550-6dc22b93e5b7
~~~
`server/analyzer/split.go:30-47`
~~~go
switch expr := expr.(type) {
case *expression.And:
return append(SplitConjunction(ctx, expr.LeftChild), SplitConjunction(ctx, expr.RightChild)...)
case *pgexprs.GMSCast:
split := SplitConjunction(ctx, expr.Child())
...
default:
return []sql.Expression{expr}
}
~~~
`server/analyzer/split.go:58-64`
~~~go
func (l *LogicTreeWalker) Next(e sql.Expression) sql.Expression {
switch expr := e.(type) {
case *pgexprs.GMSCast:
return l.Next(expr.Child())
default:
return e
}
}
~~~
`server/analyzer/init.go:129-133`
~~~go
analyzer.SplitConjunction = SplitConjunction
memo.SplitConjunction = SplitConjunction
~~~
Depends on dolthub/go-mysql-server#3591
Since we are no longer pushing filters into the join condition, some query plans have changed.
Furthermore, filters are now getting pushed down to the table node level. Filters were previously not getting pushed down to the table node level in Doltgres since GMS's
expression.SplitConjunctiondoes not recognizeGMSCastexpressions, which wrapAndexpressions. dolthub/go-mysql-server#3591 usesanalyzer.SplitConjunctioninstead, which gets replaced in Doltgres with a version ofSplitConjunctionthat handlesGMSCastexpressions.We also discovered via these query plan tests that matched filters were not getting removed (dolthub/dolt#11231). Relevant TODO and skips were added.